Allow users to specify whether to use `cabal`'s multi-repl feature #4179

fendor · 2024-04-16T16:58:31Z

We add an option to Config that allows clients to specify how HLS should load components.

The three options are:

SessionLoadSingleComponent: Always load only a single component when a new component is discovered.
SessionLoadMultipleComponents: Always allow the cradle to load multiple components at once. This might not be always possible, e.g., if the tool doesn't support multiple components loading. The cradle can decide how to handle these situations.
SessionAuto: Leave the decision to the individual cradle type. Some cradles have good support for loading multiple components, while others do not. So use multiple components loading for tools, such as cabal, and force single component loading for tools that do not support multiple components, such as stack.

This allows users to opt-out of cabal multi-repl whenever they want and vice versa.

Closes #4178

Depends on haskell/hie-bios#433

Sibling PR for vscode-haskell: haskell/vscode-haskell#1077

fendor · 2024-04-16T17:05:55Z

The only thing is kind of bad is that changing the load style requires a restart of the language server. We could track when the config changes and unload all components instead.

michaelpj · 2024-04-17T14:17:02Z

The only thing is kind of bad is that changing the load style requires a restart of the language server. We could track when the config changes and unload all components instead.

Yeah, we already do something like this when e.g. we have a change to a cabal file. Can we do the same here?

fendor · 2024-04-17T15:42:53Z

Can we do the same here?

Yeah, I believe we can.

wz1000 · 2024-04-22T07:29:00Z

I'm skeptical about the utility of the auto configuration option, but other than that it seems like it is OK to move ahead with this. We should produce an hie-bios release before merging though.

fendor · 2024-04-22T07:42:53Z

I'm skeptical about the utility of the auto configuration option, but other than that it seems like it is OK to move ahead with this. We should produce an hie-bios release before merging though.

Yes, after some more discussion, I agree that the auto feature is overkill for now. I propose the following changes to the PR:

Single Component: Default option
Multiple Components: Allow the usage of mutli-repl: True if the cabal cradle allows it.

michaelpj · 2024-04-22T08:48:23Z

Then isn't this really just a flag saying "use cabal multi-repl if available: yes/no"? I continue to be a little suspicious of pretending that it's more general than it is.

fendor · 2024-04-22T10:10:03Z

Yes it is, and the docs can make this clear, but there is nothing that is specific to cabal cradles right now. bios cradles could implement the same mechanism, we just didn't do it yet in hie-bios.

hls-plugin-api/src/Ide/Types.hs

test/testdata/schema/ghc98/default-config.golden.json

michaelpj · 2024-04-23T09:39:15Z

hls-plugin-api/src/Ide/Types.hs

@@ -194,6 +197,7 @@ instance Default Config where
    -- , cabalFormattingProvider     = "cabal-fmt"
    -- this string value needs to kept in sync with the value provided in HlsPlugins
    , maxCompletions              = 40
+    , sessionLoading              = SessionLoadSingleComponent


I do think we should make the default multi-component. We want people to benefit from this without knowing they have to turn it on! The main thing is that they can turn it off if they need to.

People have raised the concern that this is basically an untested cabal feature that may contain many hard to understand bugs, so we should disable it by default, and then enable it by default in the next release.

ghcide/session-loader/Development/IDE/Session.hs

michaelpj · 2024-04-23T09:40:59Z

ghcide/session-loader/Development/IDE/Session.hs

@@ -461,6 +469,7 @@ loadSessionWithOptions recorder SessionLoadingOptions{..} dir = do
  runningCradle <- newVar dummyAs :: IO (Var (Async (IdeResult HscEnvEq,[FilePath])))

  return $ do
+    clientConfig <- getClientConfigAction


I'm not sure this actually works? This will get the client config at this point, but we consult it inside consultCradle which is an IO function that maybe runs later? So might we just always see a stale version of the config? I think we might need to get the config in IO inside consultCradle.

It does work, in combination with invalidating the GhcSessionIO rule.

This new dependency seems to be working: https://github.com/haskell/haskell-language-server/pull/4179/files#diff-65a56362c2b3593800cd8369e6cfab1e92c347a9be97d1f0b8793f0e5dd9d524R711

And it doesn't invalidate the whole session on config changes.

ghcide/session-loader/Development/IDE/Session.hs

We add an option to `Config` that allows clients to specify how HLS should load components. We support two loading strategies: * SessionLoadSingleComponent: Always load only a single component when a new component is discovered. * SessionLoadMultipleComponents: Always allow the cradle to load multiple components at once. This might not be always possible, e.g., if the tool doesn't support multiple components loading. The cradle decides how to handle these situations. By default, we use the conservative `SessionLoadSingleComponent` mode. Additionally, changing the config at run-time leads to a reload of the GHC session, allowing users to switch between the modes without restarting the full server.

michaelpj

Thanks for tackling this!

@wz1000

## 2.9.0.0 - Bindists for GHC 9.10.1 by @wz1000, @jhrcek, @michaelpj - More hls-graph reliability improvements by @soulomoon - Refactoring of test suite runners by @soulomoon - Fixes in multiple home units support by @wz1000 ### Pull Requests - Fix quadratic memory usage in GetLocatedImports ([#4318](haskell/haskell-language-server#4318)) by @mpickering - Bump stack configs + CI to 9.6.5 and 9.8.2 ([#4316](haskell/haskell-language-server#4316)) by @jhrcek - Add support for Fourmolu 0.16 ([#4314](haskell/haskell-language-server#4314)) by @ brandonchinn178 - Code action to remove redundant record field import (fixes #4220) ([#4308](haskell/haskell-language-server#4308)) by @battermann - Use restricted monad for plugins (#4057) ([#4304](haskell/haskell-language-server#4304)) by @awjchen - 4301 we need to implement utility to wait for all runnning keys in hls graph done ([#4302](haskell/haskell-language-server#4302)) by @soulomoon - Call useWithStale instead of useWithStaleFast when calling ParseCabalFields ([#4294](haskell/haskell-language-server#4294)) by @VeryMilkyJoe - test: add test documenting #806 ([#4292](haskell/haskell-language-server#4292)) by @develop7 - ghcide: drop ghc-check and ghc-paths dependency ([#4291](haskell/haskell-language-server#4291)) by @wz1000 - Limit number of valid hole fits to 10 ([#4288](haskell/haskell-language-server#4288)) by @akshaymankar - Add common stanza to completion data ([#4286](haskell/haskell-language-server#4286)) by @VeryMilkyJoe - FindImports: ThisPkg means some home unit, not "this" unit ([#4284](haskell/haskell-language-server#4284)) by @wz1000 - Remove redudant absolutization in session loader ([#4280](haskell/haskell-language-server#4280)) by @soulomoon - Bump to new lsp versions ([#4279](haskell/haskell-language-server#4279)) by @michaelpj - Put more test code into pre-commit ([#4275](haskell/haskell-language-server#4275)) by @soulomoon - Delete library ghcide test utils ([#4274](haskell/haskell-language-server#4274)) by @soulomoon - Delete testUtil from ghcide-tests ([#4272](haskell/haskell-language-server#4272)) by @soulomoon - CI change, only run bench on performance label ([#4271](haskell/haskell-language-server#4271)) by @soulomoon - Migrate WatchedFileTests ([#4269](haskell/haskell-language-server#4269)) by @soulomoon - Migrate UnitTests ([#4268](haskell/haskell-language-server#4268)) by @soulomoon - Migrate SafeTests ([#4267](haskell/haskell-language-server#4267)) by @soulomoon - Migrate SymlinkTests ([#4266](haskell/haskell-language-server#4266)) by @soulomoon - Remove unused and outdated CHANGELOG files ([#4264](haskell/haskell-language-server#4264)) by @fendor - Enable cabal flaky test ([#4263](haskell/haskell-language-server#4263)) by @soulomoon - Migrate RootUriTests ([#4261](haskell/haskell-language-server#4261)) by @soulomoon - Migrate PreprocessorTests ([#4260](haskell/haskell-language-server#4260)) by @soulomoon - Migrate PluginSimpleTests ([#4259](haskell/haskell-language-server#4259)) by @soulomoon - Migrate ClientSettingsTests ([#4258](haskell/haskell-language-server#4258)) by @soulomoon - Unify critical session running in hls ([#4256](haskell/haskell-language-server#4256)) by @soulomoon - Bump cachix/cachix-action from 14 to 15 ([#4255](haskell/haskell-language-server#4255)) by @dependabot[bot] - Bump haskell-actions/setup from 2.7.2 to 2.7.3 ([#4254](haskell/haskell-language-server#4254)) by @dependabot[bot] - Bump haskell-actions/setup from 2.7.2 to 2.7.3 in /.github/actions/setup-build ([#4253](haskell/haskell-language-server#4253)) by @dependabot[bot] - Shorter file names completion ([#4252](haskell/haskell-language-server#4252)) by @VenInf - Fix progress start delay ([#4249](haskell/haskell-language-server#4249)) by @michaelpj - Bump cachix/install-nix-action from 26 to 27 ([#4245](haskell/haskell-language-server#4245)) by @dependabot[bot] - Bump haskell-actions/setup from 2.7.1 to 2.7.2 ([#4244](haskell/haskell-language-server#4244)) by @dependabot[bot] - Bump haskell-actions/setup from 2.7.1 to 2.7.2 in /.github/actions/setup-build ([#4243](haskell/haskell-language-server#4243)) by @dependabot[bot] - Enable test for #717 ([#4241](haskell/haskell-language-server#4241)) by @soulomoon - Remove Pepe from CODEOWNERS ([#4239](haskell/haskell-language-server#4239)) by @michaelpj - Fix resultBuilt(dirty mechanism) in hls-graph ([#4238](haskell/haskell-language-server#4238)) by @soulomoon - Support for 9.10 ([#4233](haskell/haskell-language-server#4233)) by @wz1000 - Refactor hls-test-util and reduce getCurrentDirectory after initilization ([#4231](haskell/haskell-language-server#4231)) by @soulomoon - [Migrate BootTests] part of #4173 Migrate ghcide tests to hls test utils ([#4227](haskell/haskell-language-server#4227)) by @soulomoon - Actually enable pedantic flag in ci flags job ([#4224](haskell/haskell-language-server#4224)) by @jhrcek - Cleanup cabal files, ghc compat code, fix ghc warnings ([#4222](haskell/haskell-language-server#4222)) by @jhrcek - Another attempt at using the lsp API for some progress reporting ([#4218](haskell/haskell-language-server#4218)) by @michaelpj - [Migrate diagnosticTests] part of #4173 Migrate ghcide tests to hls test utils ([#4207](haskell/haskell-language-server#4207)) by @soulomoon - Prepare release 2.8.0.0 ([#4191](haskell/haskell-language-server#4191)) by @wz1000 - Stabilize the build system by correctly house keeping the dirtykeys and rule values [flaky test #4185 #4093] ([#4190](haskell/haskell-language-server#4190)) by @soulomoon - hls-cabal-plugin: refactor context search to use `readFields` ([#4186](haskell/haskell-language-server#4186)) by @fendor - 3944 extend the properties api to better support nested configuration ([#3952](haskell/haskell-language-server#3952)) by @soulomoon ## 2.8.0.0 - Bindists for GHC 9.6.5 - New hls-notes plugin (#4126, @jvanbruegge) - Floskell, hlint and stylish-haskell plugins enabled for GHC 9.8 - Improvements for hls-graph increasing robustness (#4087, @soulomoon) - Improvements to multi-component support (#4096, #4109, #4179, @wz1000, @fendor) ### Pull Requests - Bump haskell-actions/setup from 2.7.0 to 2.7.1 ([#4189](haskell/haskell-language-server#4189)) by @dependabot[bot] - Bump haskell-actions/setup from 2.7.0 to 2.7.1 in /.github/actions/setup-build ([#4188](haskell/haskell-language-server#4188)) by @dependabot[bot] - Fix ghcdie-tests CI ([#4184](haskell/haskell-language-server#4184)) by @soulomoon - Fix ghc and hlint warnings, fix formatting ([#4181](haskell/haskell-language-server#4181)) by @jhrcek - Allow users to specify whether to use `cabal`'s multi-repl feature ([#4179](haskell/haskell-language-server#4179)) by @fendor - Improve parsing of import suggestions extending multiple multiline imports (fixes #4175) ([#4177](haskell/haskell-language-server#4177)) by @jhrcek - move ghcide-tests to haskell-language-server.cabal and make it depend on hls-test-utils ([#4176](haskell/haskell-language-server#4176)) by @soulomoon - enable ThreadId for when testing ([#4174](haskell/haskell-language-server#4174)) by @soulomoon - Drop Legacy Logger from Codebase ([#4171](haskell/haskell-language-server#4171)) by @fendor - get rid of the `unsafeInterleaveIO` at start up ([#4167](haskell/haskell-language-server#4167)) by @soulomoon - Remove EKG ([#4163](haskell/haskell-language-server#4163)) by @michaelpj - Mark plugins as not buildable if the flag is disabled ([#4160](haskell/haskell-language-server#4160)) by @michaelpj - Fix references to old CPP names in tests, update tests ([#4159](haskell/haskell-language-server#4159)) by @jhrcek - Bump haskell-actions/setup from 2.6.3 to 2.7.0 ([#4158](haskell/haskell-language-server#4158)) by @dependabot[bot] - Bump haskell-actions/setup from 2.6.3 to 2.7.0 in /.github/actions/setup-build ([#4157](haskell/haskell-language-server#4157)) by @dependabot[bot] - Remove dead code in ghcide and hls-graph for priority ([#4151](haskell/haskell-language-server#4151)) by @soulomoon - Bump haskell-actions/setup from 2.6.2 to 2.6.3 in /.github/actions/setup-build ([#4150](haskell/haskell-language-server#4150)) by @dependabot[bot] - Bump haskell-actions/setup from 2.6.2 to 2.6.3 ([#4149](haskell/haskell-language-server#4149)) by @dependabot[bot] - Run ExceptionTests in temporary directory ([#4146](haskell/haskell-language-server#4146)) by @fendor - hls-eval-plugin: Replicate #4139 ([#4140](haskell/haskell-language-server#4140)) by @mattapet - Update comment in refactor tests ([#4138](haskell/haskell-language-server#4138)) by @jhrcek - Update contact info in docs ([#4137](haskell/haskell-language-server#4137)) by @jhrcek - hls-notes-plugin: Do not error if no note is under the cursor ([#4136](haskell/haskell-language-server#4136)) by @jvanbruegge - improve logging in semantic tokens rule ([#4135](haskell/haskell-language-server#4135)) by @soulomoon - Bump softprops/action-gh-release from 1 to 2 ([#4133](haskell/haskell-language-server#4133)) by @dependabot[bot] - Bump cachix/install-nix-action from 25 to 26 ([#4132](haskell/haskell-language-server#4132)) by @dependabot[bot] - Use Set.member instead of Foldable.elem ([#4128](haskell/haskell-language-server#4128)) by @jhrcek - hls-notes-plugin: Initial implementation ([#4126](haskell/haskell-language-server#4126)) by @jvanbruegge - Enable floskell and hlint plugins for ghc 9.8 ([#4125](haskell/haskell-language-server#4125)) by @jhrcek - Integrate stylish-haskell into hls executable with ghc 9.8 ([#4124](haskell/haskell-language-server#4124)) by @jhrcek - Reduce usage of partial functions ([#4123](haskell/haskell-language-server#4123)) by @jhrcek - Benchmark: Enable 9.6, 9.8 ([#4118](haskell/haskell-language-server#4118)) by @soulomoon - Bump haskell-actions/setup from 2.6.1 to 2.6.2 in /.github/actions/setup-build ([#4116](haskell/haskell-language-server#4116)) by @dependabot[bot] - Bump haskell-actions/setup from 2.6.1 to 2.6.2 ([#4115](haskell/haskell-language-server#4115)) by @dependabot[bot] - eval: more robust way to extract comments from ParsedModule ([#4113](haskell/haskell-language-server#4113)) by @jhrcek - Improve isolation of build artefacts of test runs ([#4112](haskell/haskell-language-server#4112)) by @fendor - Improve handling of nonsense rename attempts ([#4111](haskell/haskell-language-server#4111)) by @jhrcek - Exit with non-zero exitcode if wrapper fails to launch ([#4110](haskell/haskell-language-server#4110)) by @fendor - Replace checkHomeUnitsClosed with a faster implementation ([#4109](haskell/haskell-language-server#4109)) by @wz1000 - Don't distribute gifs or plugin readmes ([#4107](haskell/haskell-language-server#4107)) by @fendor - Remove locale workaround for Module name that conatins non-ascii characters ([#4106](haskell/haskell-language-server#4106)) by @fendor - Track extra-source-files of plugins more accurately ([#4105](haskell/haskell-language-server#4105)) by @fendor - remove non-ascii name ([#4103](haskell/haskell-language-server#4103)) by @soulomoon - Add cabal-gild as a cabal file formatter plugin ([#4101](haskell/haskell-language-server#4101)) by @fendor - Remove more workarounds for GHCs < 9.2 (#4092) ([#4098](haskell/haskell-language-server#4098)) by @jhrcek - session-loader: Don't loop forever when we don't find a file in any multi component ([#4096](haskell/haskell-language-server#4096)) by @wz1000 - Prepare release 2.7.0.0 ([#4095](haskell/haskell-language-server#4095)) by @fendor - Remove more workarounds for GHCs < 9.0 ([#4092](haskell/haskell-language-server#4092)) by @jhrcek - Fix hls-graph: phantom dependencies invoke in branching deps (resolve #3423) ([#4087](haskell/haskell-language-server#4087)) by @soulomoon - Rename only if the current module compiles (#3799) ([#3848](haskell/haskell-language-server#3848)) by @sgillespie - Reintroduce ghc-lib flag for hlint plugin ([#3757](haskell/haskell-language-server#3757)) by @RaoulHC

fendor requested review from michaelpj and pepeiborra as code owners April 16, 2024 16:58

fendor force-pushed the wip/configure-multi-repl branch 2 times, most recently from 99563a5 to 35a22ad Compare April 16, 2024 17:04

fendor force-pushed the wip/configure-multi-repl branch 2 times, most recently from 989d7c1 to dce9b8b Compare April 17, 2024 11:32

fendor force-pushed the wip/configure-multi-repl branch 6 times, most recently from d3029e3 to a47b398 Compare April 23, 2024 09:34

michaelpj reviewed Apr 23, 2024

View reviewed changes

fendor force-pushed the wip/configure-multi-repl branch 4 times, most recently from 8d700ea to 095c04b Compare April 23, 2024 11:54

fendor requested review from michaelpj and wz1000 April 23, 2024 11:55

fendor force-pushed the wip/configure-multi-repl branch from 095c04b to 036a537 Compare April 23, 2024 11:56

wz1000 approved these changes Apr 23, 2024

View reviewed changes

michaelpj approved these changes Apr 23, 2024

View reviewed changes

michaelpj enabled auto-merge (squash) April 23, 2024 13:03

michaelpj merged commit a6f0008 into haskell:master Apr 23, 2024
38 checks passed

chenrui333 mentioned this pull request May 8, 2024

haskell-language-server 2.8.0.0 Homebrew/homebrew-core#171165

Merged

amesgen mentioned this pull request Jul 4, 2024

Expose lsp-haskell-session-loading config emacs-lsp/lsp-haskell#184

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow users to specify whether to use `cabal`'s multi-repl feature #4179

Allow users to specify whether to use `cabal`'s multi-repl feature #4179

fendor commented Apr 16, 2024 •

edited

Loading

fendor commented Apr 16, 2024

michaelpj commented Apr 17, 2024

fendor commented Apr 17, 2024

wz1000 commented Apr 22, 2024

fendor commented Apr 22, 2024

michaelpj commented Apr 22, 2024 •

edited

Loading

fendor commented Apr 22, 2024

michaelpj Apr 23, 2024

fendor Apr 23, 2024

michaelpj Apr 23, 2024

michaelpj Apr 23, 2024

fendor Apr 23, 2024 •

edited

Loading

michaelpj left a comment

Allow users to specify whether to use cabal's multi-repl feature #4179

Allow users to specify whether to use cabal's multi-repl feature #4179

Conversation

fendor commented Apr 16, 2024 • edited Loading

fendor commented Apr 16, 2024

michaelpj commented Apr 17, 2024

fendor commented Apr 17, 2024

wz1000 commented Apr 22, 2024

fendor commented Apr 22, 2024

michaelpj commented Apr 22, 2024 • edited Loading

fendor commented Apr 22, 2024

michaelpj Apr 23, 2024

Choose a reason for hiding this comment

fendor Apr 23, 2024

Choose a reason for hiding this comment

michaelpj Apr 23, 2024

Choose a reason for hiding this comment

michaelpj Apr 23, 2024

Choose a reason for hiding this comment

fendor Apr 23, 2024 • edited Loading

Choose a reason for hiding this comment

michaelpj left a comment

Choose a reason for hiding this comment

Allow users to specify whether to use `cabal`'s multi-repl feature #4179

Allow users to specify whether to use `cabal`'s multi-repl feature #4179

fendor commented Apr 16, 2024 •

edited

Loading

michaelpj commented Apr 22, 2024 •

edited

Loading

fendor Apr 23, 2024 •

edited

Loading